Skip to content

feat: support ellipsis placeholder expression#336

Open
hjotha wants to merge 1 commit intomendixlabs:mainfrom
hjotha:submit/microflow-ellipsis-placeholder-expression
Open

feat: support ellipsis placeholder expression#336
hjotha wants to merge 1 commit intomendixlabs:mainfrom
hjotha:submit/microflow-ellipsis-placeholder-expression

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented Apr 27, 2026

Summary

Adds ... as a microflow expression placeholder for intentionally unbound code-action parameters. This lets describe output preserve empty BasicCodeActionParameterValue bindings and lets exec write them back without inventing synthetic expression values.

Part of #332.

Changes

  • Adds ELLIPSIS to the MDL grammar and preserves it as a source expression.
  • Maps Java action arguments written as ... back to empty basic code-action parameter values.
  • Documents the syntax in the quick reference, proposal index, and Mendix microflow skill.
  • Adds a doctype example and parser/builder regression coverage.

Validation

  • make build
  • make lint-go
  • make test
  • ./bin/mxcli check mdl-examples/doctype-tests/ellipsis_placeholder.test.mdl

Symptom: microflow Java action parameters with an empty BasicCodeActionParameterValue could not be represented in MDL, so describe/exec round-trips had to either invent a value or fail to parse the generated source.

Root cause: the expression grammar had no placeholder expression that could carry an intentionally unbound code-action argument through the parser and builder.

Fix: add an ellipsis source expression, emit it for `...`, preserve it through expression formatting, and map it back to an empty BasicCodeActionParameterValue for Java action arguments.

Tests: added parser/visitor coverage, builder coverage for placeholder Java action arguments, an MDL doctype fixture, and validation docs for the new syntax.
@github-actions
Copy link
Copy Markdown

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

  • Visitor logic not explicitly shown: While the implementation appears correct (tests pass and documentation states the visitor produces ast.SourceExpr{Source: "..."} for ELLIPSIS), the diff doesn't show explicit visitor changes. Given the truncation note, this may be in the omitted sections, but it would be good to confirm the visitor correctly bridges the ELLIPSIS token to the SourceExpr AST node.

What Looks Good

  • Full-stack implementation: The feature is properly implemented through the entire pipeline:

    • Grammar: Added ELLIPSIS token and rule
    • AST: Added SourceExpr to preserve source text
    • Executor: Handles placeholder detection and conversion to empty BasicCodeActionParameterValue
    • Tests: Includes unit test verifying BSON preservation and doctype example
    • Documentation: Updated quick reference, skills doc, and added detailed proposal
  • Correct semantics: The implementation correctly preserves empty Argument values in BSON for round-trip compatibility, addressing the core motivation of maintaining symmetry in describe → exec → describe cycles.

  • Clear documentation: The proposal and accompanying documentation clearly explain that ... is round-trip-only and should not be used in newly authored scripts, preventing misuse.

  • Robust handling: The implementation correctly handles whitespace variations (e.g., " ... ") through trimming in the placeholder detection function.

  • Test coverage: Includes both unit test for the builder logic and a doctype test for end-to-end validation.

  • Minimal scope: Focused solely on adding the ellipsis placeholder for Java action parameters without attempting to over-extend to other contexts prematurely.

Recommendation

Approve. The PR thoroughly implements the ellipsis placeholder feature following all project guidelines, with proper documentation, test coverage, and full-stack implementation. The minor concern about visitor logic is likely addressed in the generated or omitted code sections given the passing tests and explicit documentation of the AST behavior.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants